Skip to content

LCORE-2920: Enhanced mock_mcp_server#2087

Open
jrobertboos wants to merge 1 commit into
lightspeed-core:mainfrom
jrobertboos:lcore-2920
Open

LCORE-2920: Enhanced mock_mcp_server#2087
jrobertboos wants to merge 1 commit into
lightspeed-core:mainfrom
jrobertboos:lcore-2920

Conversation

@jrobertboos

@jrobertboos jrobertboos commented Jul 7, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown
Contributor

Description

Expand the mock MCP server used in E2E tests to support a no-auth mode and add math tool capabilities. This enables testing MCP server interactions both with and without OAuth authentication by running two separate container instances. The server now also exposes four arithmetic tools (add, subtract, multiply, divide) with proper tools/call dispatch, making it possible to write E2E scenarios that exercise actual tool invocation through the MCP protocol.

Type of change

  • Refactor
  • New feature
  • Bug fix
  • CVE fix
  • Optimization
  • Documentation Update
  • Configuration Update
  • Bump-up service version
  • Bump-up dependent library
  • Bump-up library or tool used for development (does not change the final image)
  • CI configuration change
  • Konflux configuration change
  • Unit tests improvement
  • Integration tests improvement
  • End to end tests improvement
  • Benchmarks improvement

Tools used to create PR

Identify any AI code assistants used in this PR (for transparency and review context)

  • Assisted-by: Cursor
  • Generated by: N/A

Related Tickets & Documents

  • Related Issue # LCORE-2920
  • Closes #

Checklist before requesting a review

  • I have performed a self-review of my code.
  • PR has passed all pre-merge test jobs.
  • If it is a core feature, I have added thorough tests.

Testing

  • CI will validate the changes via pre-merge E2E test jobs.
  • Both mock-mcp (port 3000, auth required) and mock-mcp-no-auth (port 3001, no auth) services are defined in docker-compose.yaml and docker-compose-library.yaml with health checks.
  • The LCS service depends_on both mock containers, ensuring they are healthy before E2E tests begin.

Summary by CodeRabbit

  • New Features

    • Added support for running the mock MCP service without authentication.
    • Expanded mock MCP tools with addition, subtraction, multiplication, and division operations.
    • Added handling for tool execution errors, including division by zero and unknown tools.
    • Improved resilience when processing malformed requests.

  • Tests

    • Updated end-to-end test services and health checks to cover authenticated and unauthenticated modes.

@coderabbitai

coderabbitai Bot commented Jul 7, 2026 •
edited
Loading

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 2fb28f69-d01d-4140-882a-b9654526fd28

📥 Commits

Reviewing files that changed from the base of the PR and between b35bce4 and 1e1342e.

📒 Files selected for processing (4)
  • docker-compose-library.yaml
  • docker-compose.yaml
  • tests/e2e/mock_mcp_server/Dockerfile
  • tests/e2e/mock_mcp_server/server.py
📜 Recent review details
🧰 Additional context used
📓 Path-based instructions (1)
**/*

📄 CodeRabbit inference engine (Custom checks)

**/*: Flag meaningful O(n^2)+ algorithms on non-trivial inputs, including handlers and Kubernetes list operations.
Flag N+1 patterns that list items and then query once per item, including Kubernetes API and database access.
Flag expensive work inside loops, including API calls, JSON parsing, and regex compilation.
Flag unbounded growth in caches, watchers, or buffers when eviction or limits are missing.
Flag missing pagination or limits on list operations and API endpoints.
Flag secrets or tokens logged in plaintext or hardcoded in source.
Flag API endpoints missing authentication or authorization.
Flag injection vulnerabilities, including SQL injection, command injection, and path traversal.
Flag sensitive data leaked in API responses, WebSocket messages, or logs.
Flag Kubernetes Secrets and Red Hat secrets missing OwnerReferences.

Never commit secrets or keys; use environment variables for sensitive data.

Files:

  • tests/e2e/mock_mcp_server/Dockerfile
  • docker-compose-library.yaml
  • docker-compose.yaml
  • tests/e2e/mock_mcp_server/server.py
🧠 Learnings (2)
📚 Learning: 2026-05-20T08:09:30.641Z 
Learnt from: max-svistunov
Repo: lightspeed-core/lightspeed-stack PR: 1580
File: docs/design/llama-stack-config-merge/poc-results/library-mode/synthesized-run.yaml:107-110
Timestamp: 2026-05-20T08:09:30.641Z
Learning: In Llama-stack config YAMLs, when defining a Llama Guard safety shield entry, set `provider_shield_id` to the *guard model identifier* (e.g., `meta-llama/Llama-Guard-3-8B`). Do not use a chat/generative model id (e.g., `openai/gpt-4o-mini`): a chat-model id (or `native_override`) indicates only an override landed and does **not** mean the safety shield is actually gating queries. Ensure any E2E coverage for the related implementation (JIRA/E2E tests) exercises a real Llama Guard model to verify that the shield is effective.

Applied to files:

  • docker-compose-library.yaml
  • docker-compose.yaml
📚 Learning: 2026-06-24T13:45:37.249Z 
Learnt from: Jdubrick
Repo: lightspeed-core/lightspeed-stack PR: 1971
File: src/utils/markdown_repair.py:31-36
Timestamp: 2026-06-24T13:45:37.249Z
Learning: In the lightspeed-stack repository, docstrings must use the section header name "Parameters:" (not "Args:") for function arguments, even if the project references Google Python docstring conventions. Ensure docstrings follow the project’s established "Parameters:" header format for any documented function parameters.

Applied to files:

  • tests/e2e/mock_mcp_server/server.py
🔇 Additional comments (4)
tests/e2e/mock_mcp_server/server.py (1)

2-57: LGTM!

Also applies to: 70-76, 111-114, 147-157, 158-213, 216-234

tests/e2e/mock_mcp_server/Dockerfile (1)

4-5: LGTM!

docker-compose-library.yaml (1)

14-15: LGTM!

Also applies to: 95-95, 107-123

docker-compose.yaml (1)

109-110: LGTM!

Also applies to: 142-142, 154-170

Walkthrough

The E2E MCP mock server now supports authenticated and unauthenticated modes, arithmetic tools, and explicit port selection. Docker Compose starts separate mock services on ports 3000 and 3001, with lightspeed-stack waiting for the no-auth service healthcheck.

Changes

MCP mock service

Layer / File(s) Summary
Server authentication and tool behavior
tests/e2e/mock_mcp_server/server.py		 Adds optional authentication, arithmetic tool schemas and execution, tolerant JSON parsing, and MCP errors for unknown tools or division by zero.
Server startup modes
tests/e2e/mock_mcp_server/server.py, tests/e2e/mock_mcp_server/Dockerfile		 Adds --no-auth, preserves the legacy positional port, supports explicit ports, and exposes ports 3000 and 3001 through the container entrypoint.
Compose service integration
docker-compose.yaml, docker-compose-library.yaml		 Configures authenticated and unauthenticated mock services on separate ports and adds a healthy dependency from lightspeed-stack to mock-mcp-no-auth.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Compose
  participant lightspeed-stack
  participant mock-mcp-no-auth
  Compose->>mock-mcp-no-auth: Start on port 3001 with --no-auth
  mock-mcp-no-auth-->>Compose: Report healthy from /health
  Compose->>lightspeed-stack: Start after dependency is healthy
  lightspeed-stack->>mock-mcp-no-auth: Send MCP tool request
  mock-mcp-no-auth-->>lightspeed-stack: Return tool result or MCP error
Loading
🚥 Pre-merge checks | ✅ 7
✅ Passed checks (7 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title is related to the main change, identifying the mock_mcp_server enhancement, though it is broad and not very specific.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Performance And Algorithmic Complexity ✅ Passed server.py does only constant-size per-request work; tools/list returns a fixed list, tools/call uses a 4-entry dict, and no nested loops/unbounded buffers appear.
Security And Secret Handling ✅ Passed No plaintext secrets, token logging, command/path injection, or K8s secret changes; the no-auth mode is confined to the dedicated E2E mock service.
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
✨ Simplify code
  • Create PR with simplified code

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Checkov (3.3.8)
docker-compose-library.yaml

Traceback (most recent call last):
File "/usr/local/bin/checkov", line 2, in
from checkov.main import Checkov
ModuleNotFoundError: No module named 'checkov'

docker-compose.yaml

Traceback (most recent call last):
File "/usr/local/bin/checkov", line 2, in
from checkov.main import Checkov
ModuleNotFoundError: No module named 'checkov'

tests/e2e/mock_mcp_server/Dockerfile

Traceback (most recent call last):
File "/usr/local/bin/checkov", line 2, in
from checkov.main import Checkov
ModuleNotFoundError: No module named 'checkov'

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jrobertboos
enhance(e2e): expanded mock_mcp_server
1e1342e 
- added functional tools to mock_mcp
- added mock_mcp that does not require auth
@jrobertboos
jrobertboos marked this pull request as ready for review July 20, 2026 14:13
Jazzcort
Jazzcort approved these changes Jul 20, 2026
View reviewed changes

@Jazzcort Jazzcort left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer
@Jazzcort Jazzcort Jazzcort approved these changes
Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jrobertboos @Jazzcort